Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

migrate to new platform-cache-workflow to manage Dockerfile caching in CI #1703

Merged
merged 3 commits into from
Oct 3, 2023

Conversation

spoonincode
Copy link
Member

@spoonincode spoonincode commented Oct 2, 2023

Migrate from existing in-repo platforms.yaml+discover-platforms-action to a new platform-cache-workflow. Work on reproducible builds was primary motivator for change; I want to separate out the concept of caching platform build environments away from what the combinations of platforms leap builds with (since platforms may no longer be 1:1 with what gets built and tested).

Besides this additional encapsulation, new improvements/benefits are,

  • Includes work done in don't skip the "build platforms" job even when there are no platforms to build #283 (which I will close now). This gets rid of all the obnoxious if: always() nonsense and makes the workflows cancelable again(!), but comes at the price of some potential issues with ENF runners that will just have to be evaluated in practice.
  • Removes platforms.json and expects to be passed platform files as a workflow parameter; directories are allowed where it will discover all files in the directory and use each file found as a platform (as being done here in this PR). The base filename of the file will be used as the platform name.
  • Allows a target to be specified on the dockerfile as the stage to use and cache. This will be a requirement for the reproducible build, where ultimately the current
platform-files: .cicd/platforms

will become

platform-files: |
  .cicd/platforms
  tools/reproducible.Dockerfile:build

So even though reproducible.Dockerfile defines multiple stages, only the build stage is used and cached.

work on #1641

@spoonincode spoonincode added the CICD Anything dealing with the CI workflow behavior label Oct 2, 2023
@spoonincode spoonincode linked an issue Oct 2, 2023 that may be closed by this pull request
Copy link
Member

@heifner heifner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@@ -79,14 +79,13 @@ jobs:

dev-package:
name: Build leap-dev package
needs: [platforms, build-base]
if: always() && needs.platforms.result == 'success' && needs.build-base.result == 'success'
needs: [platform-cache, build-base]
strategy:
fail-fast: false
matrix:
platform: [ubuntu20, ubuntu22]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought, while doing this update should we update the platforms in the matrix: sections throughout our workflows to be dynamic from the platform-cache-workflow like done in ph_backward_compatibility?

platform: ${{fromJSON(needs.platform-cache.outputs.platform-list)}}

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like that is the right approach now, but that's only because there is a 1:1 between all the platform files and all the platforms we're going to build leap-dev on. That may not always be true. For example, we may choose not to support building on Ubuntu 20, but still support running the reproducible build on Ubuntu 20. In such a scenario we'd still need a ubuntu20 platform defined in the platforms/platform-list, but its purpose would only be for testing. So using platform-list here wouldn't work.

(I expected to already need to make use of this for the reproducible builds: I expected I'd need a special ubuntu20+cmake3.27 platform to run ctest on since cmake3.27 is what is used to build. Shockingly, that doesn't appear to be the case)

I feel like we probably need some sort of .json file that gets sucked in at the start of the workflow that defines these copy pasted repetitive items like [ubuntu20, ubuntu22]. (It gets more unwieldy with the upcoming reproducible workflow additions)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh, hm, I see I did plumb that through elsewhere as platform-list..

platform-list: '["${{github.event.inputs.platform-choice}}"]'

platform-list: ${{needs.platform-cache.outputs.platform-list}}

In this case here, I guess it could be changed to platform-list for consistency

Copy link
Contributor

@oschwaldp-oci oschwaldp-oci Oct 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually those are from the work I did earlier to use platform-matrix which you renamed to platform-list in this PR with your move to the platform-cache-workflow.

I'm fine with leaving the [ubuntu20, ubuntu22] was just asking since this PR was working in that space. You called out a good example where this won't be 1:1, so that's good enough of a reason to leave it. I just wish there was a way to not have it hardcoded everywhere. It makes updating the workflows a bit tedious and error prone when adopting or dropping a platform.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should come up with something better; but at least let's look at how the reproducible one works before noodling on it more. (and, at the danger of overthinking future proofing, how something like an ARM build or a macOS build would look)

@oschwaldp-oci
Copy link
Contributor

Had trouble running the ph_backward_compatibility workflow from this branch.
https://github.com/AntelopeIO/leap/actions/runs/6387705229/job/17336421454#step:3:9

@spoonincode
Copy link
Member Author

filled out PR description some more. proper docs in https://github.com/AntelopeIO/platform-cache-workflow will need to wait another day or two

@spoonincode spoonincode merged commit 19aea6f into main Oct 3, 2023
31 checks passed
@spoonincode spoonincode deleted the platform-cache-workflow branch October 3, 2023 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CICD Anything dealing with the CI workflow behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

reproducible binary builds
3 participants